Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix VS2019 16.10 build error #6

Merged
merged 2 commits into from
Jun 10, 2021
Merged

Conversation

fw2568
Copy link

@fw2568 fw2568 commented Jun 10, 2021

No description provided.

@fw2568 fw2568 merged commit 302d3e5 into dbosoft-master Jun 10, 2021
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
IP fragmentation engine may not only steal the packet but also add
more.  For example, after receiving the last fragment, it will
add all previous fragments to a batch.  Unfortunately, it will also
free the original last fragment and replace it with a copy.
This invalidates the 'packet_clone' pointer in the dpif_netdev_execute()
leading to the use-after-free:

==3525086==ERROR: AddressSanitizer: heap-use-after-free on
                  address 0x61600020439c at pc 0x000000688a6d
READ of size 1 at 0x61600020439c thread T0
    #0 0x688a6c in dp_packet_swap ./lib/dp-packet.h:265:5
    #1 0x68781d in dpif_netdev_execute lib/dpif-netdev.c:4103:9
    #2 0x6675db in dpif_netdev_operate lib/dpif-netdev.c:4129:25
    #3 0x691e5e in dpif_operate lib/dpif.c:1367:13
    #4 0x692909 in dpif_execute lib/dpif.c:1321:9
    #5 0x5b19c6 in packet_execute ofproto/ofproto-dpif.c:4991:5
    #6 0x5a2861 in ofproto_packet_out_finish ofproto/ofproto.c:3662:5
    #7 0x5a65c6 in do_bundle_commit ofproto/ofproto.c:8270:13
    #8 0x5a0cae in handle_bundle_control ofproto/ofproto.c:8309:17
    #9 0x59a476 in handle_single_part_openflow ofproto/ofproto.c:8593:16
    #10 0x5877ac in handle_openflow ofproto/ofproto.c:8674:21
    #11 0x6296f1 in ofconn_run ofproto/connmgr.c:1329:13
    #12 0x62925d in connmgr_run ofproto/connmgr.c:356:9
    #13 0x586904 in ofproto_run ofproto/ofproto.c:1879:5
    #14 0x55c830 in bridge_run__ vswitchd/bridge.c:3251:9
    #15 0x55c015 in bridge_run vswitchd/bridge.c:3310:5
    #16 0x575f31 in main vswitchd/ovs-vswitchd.c:127:9
    #17 0x7f01099d3492 in __libc_start_main (/lib64/libc.so.6+0x23492)
    #18 0x47d96d in _start (vswitchd/ovs-vswitchd+0x47d96d)

0x61600020439c is located 28 bytes inside of 560-byte region
freed by thread T0 here:
    #0 0x5177a8 in free (vswitchd/ovs-vswitchd+0x5177a8)
    #1 0x6b17b6 in dp_packet_delete ./lib/dp-packet.h:256:9
    #2 0x6afeee in ipf_extract_frags_from_batch lib/ipf.c:947:17
    #3 0x6afd63 in ipf_preprocess_conntrack lib/ipf.c:1232:9
    #4 0x946b2c in conntrack_execute lib/conntrack.c:1446:5
    #5 0x67e3ed in dp_execute_cb lib/dpif-netdev.c:8277:9
    #6 0x7097d7 in odp_execute_actions lib/odp-execute.c:865:17
    #7 0x66409e in dp_netdev_execute_actions lib/dpif-netdev.c:8322:5
    #8 0x6877ad in dpif_netdev_execute lib/dpif-netdev.c:4090:5
    #9 0x6675db in dpif_netdev_operate lib/dpif-netdev.c:4129:25
    #10 0x691e5e in dpif_operate lib/dpif.c:1367:13
    #11 0x692909 in dpif_execute lib/dpif.c:1321:9
    #12 0x5b19c6 in packet_execute ofproto/ofproto-dpif.c:4991:5
    #13 0x5a2861 in ofproto_packet_out_finish ofproto/ofproto.c:3662:5
    #14 0x5a65c6 in do_bundle_commit ofproto/ofproto.c:8270:13
    #15 0x5a0cae in handle_bundle_control ofproto/ofproto.c:8309:17
    #16 0x59a476 in handle_single_part_openflow ofproto/ofproto.c:8593:16
    #17 0x5877ac in handle_openflow ofproto/ofproto.c:8674:21
    #18 0x6296f1 in ofconn_run ofproto/connmgr.c:1329:13
    #19 0x62925d in connmgr_run ofproto/connmgr.c:356:9
    #20 0x586904 in ofproto_run ofproto/ofproto.c:1879:5
    #21 0x55c830 in bridge_run__ vswitchd/bridge.c:3251:9
    #22 0x55c015 in bridge_run vswitchd/bridge.c:3310:5
    #23 0x575f31 in main vswitchd/ovs-vswitchd.c:127:9
    #24 0x7f01099d3492 in __libc_start_main (/lib64/libc.so.6+0x23492)

The issue can be reproduced with system-userspace testsuite on the
'conntrack - IPv4 fragmentation with fragments specified' test.
Previously, there was a leak inside the IP fragmentation module that
kept the original segment, so 'packet_clone' remained a valid pointer.
But commit 803ed12 ("ipf: release unhandled packets from the batch")
fixed the leak leading to use-after-free.

Using the packet from a batch instead of 'packet_clone' to swap packet
content to avoid the issue.

While investigating this problem, more issues uncovered.  One of them
is that IP fragmentation engine can add more packets to the batch, but
there is no way to get them to a caller.  Adding an extra branch for
this case with a 'FIXME' comment in order to highlight the issue.

Another one is that IP fragmentation engine will keep only 32 fragments
dropping all other fragments while refilling a batch, but that should
be fixed separately.

Fixes: 7e6b41a ("dpif-netdev: Fix crash when PACKET_OUT is metered.")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
This commit improves handling of packets where the allocated memory
is less than 64 bytes.  For packets recevied from DPDK ports this
never matters, as an mbuf always pre-allocates enough space, however
this can occur in cases where packet received from a kernel interface
or injected by an OpenFlow controller.  The fix is required to
ensure OVS doesn't overread the allocated memory, e.g.:

 ==49944==ERROR: AddressSanitizer: heap-buffer-overflow on address
 0x6060000d8181 at pc 0x000001cb9d24 bp 0x7ffce3b385d0 sp 0x7ffce3b385c8
 READ of size 64 at 0x6060000d8181 thread T0
    #0 0x1cb9d23 in mfex_avx512_process lib/dpif-netdev-extract-avx512.c:491:26
    #1 0x1cb9d23 in mfex_avx512_ip_udp lib/dpif-netdev-extract-avx512.c:625:1
    #2 0x18786a1 in dpif_miniflow_extract_autovalidator lib/dpif-netdev-private-extract.c:277:29
    #3 0x1cbca5c in dp_netdev_input_outer_avx512 lib/dpif-netdev-avx512.c:159:19
    #4 0x1853048 in dp_netdev_process_rxq_port lib/dpif-netdev.c:4900:19
    #5 0x1837c76 in dpif_netdev_run lib/dpif-netdev.c:6197:25
    #6 0x1727a02 in type_run ofproto/ofproto-dpif.c:370:9
    #7 0x16f6e07 in ofproto_type_run ofproto/ofproto.c:1778:31
    #8 0x16c1a8b in bridge_run__ vswitchd/bridge.c:3245:9
    #9 0x16bd2fd in bridge_run vswitchd/bridge.c:3310:5
    #10 0x16db8fe in main vswitchd/ovs-vswitchd.c:127:9
    #11 0x7fbc0c5b61a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #12 0xedabbd in _start (vswitchd/ovs-vswitchd+0xedabbd)

 0x6060000d8181 is located 9 bytes to the right of 56-byte
                region [0x6060000d8140,0x6060000d8178)
 allocated by thread T0 here:
    #0 0xf7b09f in malloc (vswitchd/ovs-vswitchd+0xf7b09f)
    #1 0x1aff3b9 in xmalloc__ lib/util.c:137:15
    #2 0x1aff3b9 in xmalloc lib/util.c:172:12
    #3 0x1afe211 in process_command lib/unixctl.c:310:13
    #4 0x1afe211 in run_connection lib/unixctl.c:344:17
    #5 0x1afe211 in unixctl_server_run lib/unixctl.c:395:21
    #6 0x16db918 in main vswitchd/ovs-vswitchd.c:128:9
    #7 0x7fbc0c5b61a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

The solution implemented uses a mask-to-zero if the available buffer
size is less than 64 bytes, and a branch for which type of load is used.

Fixes: 250cedd ("dpif-netdev/mfex: Add AVX512 based optimized miniflow extract")
Reported-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
Found by AddressSanitizer when running OVN tests:
  Direct leak of 64 byte(s) in 1 object(s) allocated from:
      #0 0x498fb2 in calloc (/ic/ovn-ic+0x498fb2)
      #1 0x5f681e in xcalloc__ ovs/lib/util.c:121:31
      #2 0x5f681e in xzalloc__ ovs/lib/util.c:131:12
      #3 0x5f681e in xzalloc ovs/lib/util.c:165:12
      #4 0x5e3697 in ovsdb_idl_txn_add_map_op ovs/lib/ovsdb-idl.c:4057:29
      #5 0x4d3f25 in update_isb_pb_external_ids ic/ovn-ic.c:576:5
      #6 0x4cc4cc in create_isb_pb ic/ovn-ic.c:716:5
      #7 0x4cc4cc in port_binding_run ic/ovn-ic.c:803:21
      #8 0x4cc4cc in ovn_db_run ic/ovn-ic.c:1700:5
      #9 0x4c9c1c in main ic/ovn-ic.c:1984:17
      #10 0x7f9ad9f4a0b2 in __libc_start_main

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
UB Sanitizer report:
  lib/stopwatch.c:119:22: runtime error: index 18446744073709551615 out of
                          bounds for type 'long long unsigned int [50]'
      #0 0x698358 in calc_percentile lib/stopwatch.c:119
      #1 0x69ada1 in add_sample lib/stopwatch.c:231
      #2 0x69c086 in stopwatch_end_sample_protected lib/stopwatch.c:386
      #3 0x69c522 in stopwatch_thread lib/stopwatch.c:441
      #4 0x684bae in ovsthread_wrapper lib/ovs-thread.c:383
      #5 0x7f042838b298 in start_thread (/lib64/libpthread.so.0+0x9298)
      #6 0x7f04277f2352 in clone (/lib64/libc.so.6+0x100352)

Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
Reported by UndefinedBehaviorSanitizer:
  tests/idltest.c:3602:12:
        runtime error: member access within null pointer of type
                       'const struct idltest_simple'
      #0 0x4295af in idltest_simple_cursor_first_ge tests/idltest.c:3602
      #1 0x41c81b in test_idl_compound_index_single_column tests/test-ovsdb.c:3128
      #2 0x41e035 in do_idl_compound_index tests/test-ovsdb.c:3277
      #3 0x4cf640 in ovs_cmdl_run_command__ lib/command-line.c:247
      #4 0x4cf79f in ovs_cmdl_run_command lib/command-line.c:278
      #5 0x4072f7 in main tests/test-ovsdb.c:79
      #6 0x7fa858675b74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
      #7 0x4060ed in _start (/root/ovs/tests/test-ovsdb+0x4060ed)

Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
When compiled with clang and '-fsanitize=undefined' set, running
'ovsdb-client --timestamp monitor Open_vSwitch' in a sandbox triggers
the following undefined behavior (flagged by UBSan):

  lib/dynamic-string.c:207:38: runtime error: applying zero offset to null pointer
      #0 0x4ebc18 in ds_put_strftime_msec lib/dynamic-string.c:207:38
      #1 0x4ebd04 in xastrftime_msec lib/dynamic-string.c:225:5
      #2 0x552e6a in table_format_timestamp__ lib/table.c:226:12
      #3 0x552852 in table_print_timestamp__ lib/table.c:233:27
      #4 0x5506f3 in table_print_table__ lib/table.c:254:5
      #5 0x550633 in table_format lib/table.c:601:9
      #6 0x5524f3 in table_print lib/table.c:633:5
      #7 0x44dc5e in monitor_print_table ovsdb/ovsdb-client.c:1019:5
      #8 0x44c650 in monitor_print ovsdb/ovsdb-client.c:1040:13
      #9 0x44ac56 in do_monitor__ ovsdb/ovsdb-client.c:1500:21
      #10 0x44636e in do_monitor ovsdb/ovsdb-client.c:1575:5
      #11 0x442c41 in main ovsdb/ovsdb-client.c:283:5

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568 pushed a commit that referenced this pull request Sep 7, 2022
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in
 lib/dpif-netlink.c:1077:40: runtime error:
   left shift of 1 by 31 places cannot be represented in type 'int'

     #0  0x73fc31 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1077:40
     #1  0x73fc31 in dpif_netlink_port_add lib/dpif-netlink.c:1132:17
     #2  0x2c1745 in dpif_port_add lib/dpif.c:597:13
     #3  0x07b279 in port_add ofproto/ofproto-dpif.c:3957:17
     #4  0x01b209 in ofproto_port_add ofproto/ofproto.c:2124:13
     #5  0xfdbfce in iface_do_create vswitchd/bridge.c:2066:13
     #6  0xfdbfce in iface_create vswitchd/bridge.c:2109:13
     #7  0xfdbfce in bridge_add_ports__ vswitchd/bridge.c:1173:21
     #8  0xfb5319 in bridge_add_ports vswitchd/bridge.c:1189:5
     #9  0xfb5319 in bridge_reconfigure vswitchd/bridge.c:901:9
     #10 0xfae0f9 in bridge_run vswitchd/bridge.c:3334:9
     #11 0xfe67dd in main vswitchd/ovs-vswitchd.c:129:9
     #12 0x4b6d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
     #13 0x4b6e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
     #14 0x562594eed024 in _start (vswitchd/ovs-vswitchd+0x787024)

Fixes: 526df7d ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others")
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
fw2568 pushed a commit that referenced this pull request Nov 1, 2022
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior in
 lib/dpif-netlink.c:1077:40: runtime error:
   left shift of 1 by 31 places cannot be represented in type 'int'

     #0  0x73fc31 in dpif_netlink_port_add_compat lib/dpif-netlink.c:1077:40
     #1  0x73fc31 in dpif_netlink_port_add lib/dpif-netlink.c:1132:17
     #2  0x2c1745 in dpif_port_add lib/dpif.c:597:13
     #3  0x07b279 in port_add ofproto/ofproto-dpif.c:3957:17
     #4  0x01b209 in ofproto_port_add ofproto/ofproto.c:2124:13
     #5  0xfdbfce in iface_do_create vswitchd/bridge.c:2066:13
     #6  0xfdbfce in iface_create vswitchd/bridge.c:2109:13
     #7  0xfdbfce in bridge_add_ports__ vswitchd/bridge.c:1173:21
     #8  0xfb5319 in bridge_add_ports vswitchd/bridge.c:1189:5
     #9  0xfb5319 in bridge_reconfigure vswitchd/bridge.c:901:9
     #10 0xfae0f9 in bridge_run vswitchd/bridge.c:3334:9
     #11 0xfe67dd in main vswitchd/ovs-vswitchd.c:129:9
     #12 0x4b6d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
     #13 0x4b6e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
     #14 0x562594eed024 in _start (vswitchd/ovs-vswitchd+0x787024)

Fixes: 526df7d ("tunnel: Provide framework for tunnel extensions for VXLAN-GBP and others")
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
@fw2568 fw2568 deleted the pull-upstream/vs2019_error branch November 1, 2022 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant